Fix invalid headers of multiple cookie and set-cookie by chenBright · Pull Request #2577 · apache/brpc

chenBright · 2024-03-19T16:32:57Z

What problem does this PR solve?

Issue Number: resolve #2575

Problem Summary:

目前bRPC处理cookie和set-cookie这两个header的逻辑有问题：

RFC 9114规定：多个cookie不能用,分隔，应该要用; ；

If a decompressed field section contains multiple cookie field lines, these MUST be concatenated into a single byte string using the two-byte delimiter of "; " (ASCII 0x3b, 0x20) before being passed into a context other than HTTP/2 or HTTP/3, such as an HTTP/1.1 connection, or a generic HTTP server application.

RFC 6265 规定：多个set-cookie不能合并到一个header。

Origin servers SHOULD NOT fold multiple Set-Cookie header fields into
a single header field. The usual mechanism for folding HTTP headers
fields (i.e., as defined in [RFC2616]) might change the semantics of
the Set-Cookie header field because the %x2C (",") character is used
by Set-Cookie in a way that conflicts with such folding.

What is changed and the side effects?

Changed:

多个cookie要用; 分隔；
内部使用std::unordered_multimap代替butil::CaseIgnoredFlatMap，用来支持同时保存多个set-cookie的特性。为了尽量兼容原来的逻辑：
2.1 GetHeader、SetHeader只对第一个set-cookie生效。当只有一个set-cookie的时候，这两个api的表现不变。
2.2 AppendHeader会直接创建新的set-cookie。
2.3 HeaderBegin和HeaderEnd这两个api表现不变，还是可以遍历到所有header。

Side effects:

Performance effects(性能影响):
Breaking backward compatibility(向后兼容性):

Check List:

Please make sure your changes are compilable(请确保你的更改可以通过编译).
When providing us with a new feature, it is best to add related tests(如果你向我们增加一个新的功能, 请添加相关测试).
Please follow Contributor Covenant Code of Conduct.(请遵循贡献者准则).

chenBright · 2024-03-31T12:42:30Z

@wwbmmm 有空看看这个PR

wwbmmm · 2024-04-01T03:05:26Z

这个改动可能会影响性能？有没有和原来的实现做对比

chenBright · 2024-04-11T09:59:08Z

这个改动可能会影响性能？有没有和原来的实现做对比

unordered_multimap性能肯定比FlatMap性能差。另外，上面通过遍历来获取key对应的所有value的方法也是比较低效的。或许，实现一个multi版本的FlatMap更合适。

chenBright · 2024-04-14T06:11:58Z

在FlatMap的基础上支持Multi，Multi FlatMap性能跟FlatMap相当，比std::unordered_(multi)map好一些。以下是在本地运行FlatMapTest.perf得到的性能比较：

[ value = 8 bytes ]
Sequentially inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/300/290/2010/210/230
Sequentially erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/1700/150/160/170/250
Sequentially inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 16/15/360/342/206/195/219
Sequentially erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/18/178/159/149/159/149
Sequentially inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/15/415/410/200/192/235
Sequentially erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 14/17/201/181/151/181/154
[ value = 32 bytes ]
Sequentially inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/280/280/250/200/230
Sequentially erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/2070/150/160/160/160
Sequentially inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 17/17/330/329/207/185/212
Sequentially erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/17/172/163/146/157/148
Sequentially inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 17/17/405/406/197/185/215
Sequentially erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/16/206/188/158/168/159
[ value = 128 bytes ]
Sequentially inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/30/290/290/420/220/250
Sequentially erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/180/150/160/160/160
Sequentially inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 22/25/352/349/213/193/222
Sequentially erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/17/170/165/160/171/157
Sequentially inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 21/24/416/422/210/206/242
Sequentially erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 16/16/213/190/163/171/159
[ value = 8 bytes ]
Randomly inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/290/260/250/220/220
Randomly erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/240/220/170/170/180
Randomly inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 16/15/315/309/206/191/215
Randomly erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/17/258/240/155/193/156
Randomly inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/16/378/363/208/191/210
Randomly erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 14/16/311/290/162/187/169
[ value = 32 bytes ]
Randomly inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/280/270/240/230/220
Randomly erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 10/20/250/220/170/180/160
Randomly inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 18/18/310/304/209/192/209
Randomly erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/16/255/247/155/175/152
Randomly inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 17/17/381/367/209/197/214
Randomly erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/17/310/296/163/188/168
[ value = 128 bytes ]
Randomly inserting 100 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 30/40/300/290/280/230/230
Randomly erasing 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/20/230/220/160/180/170
Randomly inserting 1000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 29/33/327/329/219/197/220
Randomly erasing 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 14/16/257/247/159/182/156
Randomly inserting 10000 into FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 35/39/398/400/220/213/246
Randomly erasing 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 35/36/330/319/221/224/200
[ value = 8 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 10/20/140/130/60/70/50
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/172/161/77/54/46
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/16/216/211/73/56/51
[ value = 32 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 10/20/130/130/70/60/50
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/174/163/73/54/49
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/218/217/75/58/52
[ value = 128 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/140/130/80/50/60
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/173/171/73/55/49
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 26/22/238/252/107/89/91
[ value = 8 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/140/130/70/50/60
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/14/180/160/68/57/47
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/221/210/72/57/51
[ value = 32 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 20/10/140/130/70/60/50
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/167/160/69/53/50
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 15/14/224/219/75/59/52
[ value = 128 bytes ]
Seeking 100 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 10/10/140/130/80/50/60
Seeking 1000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 13/13/170/167/70/54/52
Seeking 10000 from FlatMap/MultiFlatMap/std::map/butil::PooledMap/std::unordered_map/std::unordered_multimap/butil::hash_map takes 26/22/238/240/85/70/67

wwbmmm · 2024-04-23T09:15:57Z

LGTM

chenBright · 2024-05-07T15:35:53Z

解决与主干的冲突

chenBright force-pushed the cookie branch from 7863005 to 30ed3aa Compare March 19, 2024 18:23

chenBright mentioned this pull request Mar 20, 2024

http协议set-cookie header格式不符合mdn文档要求 #2575

Closed

wwbmmm reviewed Apr 1, 2024

View reviewed changes

Comment thread src/brpc/http_header.h

Comment thread src/brpc/http_header.cpp

chenBright force-pushed the cookie branch 4 times, most recently from 7abda2b to 777e67c Compare April 14, 2024 05:54

wwbmmm reviewed Apr 22, 2024

View reviewed changes

Comment thread src/brpc/http_header.h Outdated

chenBright force-pushed the cookie branch from 777e67c to 1b0ad36 Compare April 22, 2024 12:12

chenBright added 3 commits May 7, 2024 17:44

Fix invalid headers of multiple cookie and set-cookie

c25f114

Support MultiFlatMap

fb7e8e7

Remove unused header

0a95f0c

chenBright force-pushed the cookie branch from 1b0ad36 to 0a95f0c Compare May 7, 2024 14:31

wwbmmm merged commit 4f85335 into apache:master Jun 3, 2024

chenBright deleted the cookie branch June 11, 2024 01:59

chenBright mentioned this pull request Jun 21, 2024

Fix multi FlatMap scale and size #2669

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix invalid headers of multiple cookie and set-cookie#2577

Fix invalid headers of multiple cookie and set-cookie#2577
wwbmmm merged 3 commits into
apache:masterfrom
chenBright:cookie

chenBright commented Mar 19, 2024 •

edited

Loading

Uh oh!

chenBright commented Mar 31, 2024

Uh oh!

wwbmmm commented Apr 1, 2024

Uh oh!

Uh oh!

Uh oh!

chenBright commented Apr 11, 2024

Uh oh!

chenBright commented Apr 14, 2024 •

edited

Loading

Uh oh!

Uh oh!

wwbmmm commented Apr 23, 2024

Uh oh!

chenBright commented May 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chenBright commented Mar 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and the side effects?

Check List:

Uh oh!

chenBright commented Mar 31, 2024

Uh oh!

wwbmmm commented Apr 1, 2024

Uh oh!

Uh oh!

Uh oh!

chenBright commented Apr 11, 2024

Uh oh!

chenBright commented Apr 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

wwbmmm commented Apr 23, 2024

Uh oh!

chenBright commented May 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chenBright commented Mar 19, 2024 •

edited

Loading

chenBright commented Apr 14, 2024 •

edited

Loading